• X
• Mastodon
• LinkedIn
• Facebook

backend/core-api/src/commands/migrateContentType.ts (1)

63-64: Avoid process.exit(); close the Mongo client in a finally block and let Node exit naturally.

Prevents abrupt termination and ensures connections/logs flush.

Apply:

-  console.log(`Process finished at: ${new Date().toISOString()}`);
-
-  process.exit();
+  console.log(`Process finished at: ${new Date().toISOString()}`);
};
 
-command();
+command()
+  .catch((e) => {
+    console.error(`Fatal error: ${e?.message || e}`);
+    process.exitCode = 1;
+  })
+  .finally(async () => {
+    try {
+      await client.close();
+    } catch {}
+  });

backend/core-api/src/commands/migrateTags.ts (1)

34-41: Close the client in finally; don’t rely on process.exit().

Current flow closes the client only on error and then calls process.exit() on success.

Apply:

-  } catch (e) {
-    console.log(`Error occurred: ${e.message}`);
-    await client.close();
-  }
-
-  console.log(`Process finished at: ${new Date().toISOString()}`);
-
-  process.exit();
+  } catch (e) {
+    console.error(`Error occurred: ${e.message}`);
+    process.exitCode = 1;
+  } finally {
+    try {
+      await client.close();
+    } catch {}
+    console.log(`Process finished at: ${new Date().toISOString()}`);
+  }

backend/core-api/src/modules/tags/utils.ts (3)

29-33: Avoid refetch when you already have parentTag; recurse via parentTag to save one read.

Function only needs the ancestor chain, not the updated relatedIds snapshot.

Apply:

-  const updated = await models.Tags.findOne({ _id: tag.parentId });
-
-  if (updated) {
-    await setRelatedTagIds(models, updated);
-  }
+  if (parentTag.parentId) {
+    const grand = await models.Tags.findOne({ _id: parentTag.parentId });
+    if (grand) await setRelatedTagIds(models, grand);
+  }

---

47-50: Avoid mutating the input tag’s relatedIds reference.

Clone before push to prevent unintended side effects.

Apply:

-  const relatedIds: string[] = tag.relatedIds || [];
-
-  relatedIds.push(tag._id);
+  const relatedIds: string[] = [...(tag.relatedIds || []), tag._id];

---

58-67: Remove unnecessary async in forEach and prefer for..of for clarity.

No awaits inside; async adds confusion.

Apply:

-  tags.forEach(async (t) => {
-    const ids = (t.relatedIds || []).filter((id) => !relatedIds.includes(id));
-    doc.push({
-      updateOne: {
-        filter: { _id: t._id },
-        update: { $set: { relatedIds: ids } },
-      },
-    });
-  });
+  for (const t of tags) {
+    const ids = (t.relatedIds || []).filter((id) => !relatedIds.includes(id));
+    doc.push({
+      updateOne: {
+        filter: { _id: t._id },
+        update: { $set: { relatedIds: ids } },
+      },
+    });
+  }

backend/erxes-api-shared/src/core-types/modules/tags/tag.ts (1)

7-9: Align document shape with schema timestamps and clarify optionality.

Add updatedAt to ITagDocument (timestamps: true). Keep isGroup optional on input (ITag) but it’s always present on documents.

 export interface ITagDocument extends ITag, Document {
   _id: string;
   createdAt: Date;
+  updatedAt: Date;
 }

backend/core-api/src/modules/tags/graphql/queries.ts (1)

26-41: Avoid shadowing params and improve readability.

Local ids shadows params.ids; rename and keep intent clear.

-    let ids = [parentId, ...parentTag];
+    let descendantIds = [parentId, ...parentTag];
@@
-      if (childTag.length > 0) {
-        ids = [...ids, ...childTag];
+      if (childTag.length > 0) {
+        descendantIds = [...descendantIds, ...childTag];
         await getChildTags(childTag);
       }
@@
-    filter._id = { $in: ids };
+    filter._id = { $in: descendantIds };

backend/core-api/src/modules/tags/db/definitions/tags.ts (1)

11-12: Label may mislead: these are descendant IDs, not just direct children.

setRelatedTagIds propagates transitive IDs upward. Consider renaming the label to avoid confusion.

-      relatedIds: { type: [String], label: 'Children tag ids' },
+      relatedIds: { type: [String], label: 'Descendant tag ids' },

backend/core-api/src/modules/tags/graphql/schemas.ts (2)

43-43: Prefer an input object for mutation params

Using an input object improves evolvability (additive changes, deprecations) vs. spreading scalars.

Proposed additions outside this hunk:

--- a/backend/core-api/src/modules/tags/graphql/schemas.ts
+++ b/backend/core-api/src/modules/tags/graphql/schemas.ts
@@
 export const types = `
   type Tag @key(fields: "_id") @cacheControl(maxAge: 3) {
@@
   }
+
+  input TagMutationParams {
+    colorCode: String
+    parentId: String
+    isGroup: Boolean
+  }
 `;
 
-const mutationParams = `
-  colorCode: String,
-  parentId: String,
-  isGroup: Boolean,
-`;
+const mutationParams = `params: TagMutationParams`;

---

4-13: Make _id and name non-nullable; verify Date scalar registration

Mark _id and name as String! if always present, and confirm the Date scalar is registered globally (e.g. via apolloCommonTypes/apolloCustomScalars). Keep createdAt: Date nullable only if older records may lack it.

   type Tag @key(fields: "_id") @cacheControl(maxAge: 3) {
-    _id: String
-    name: String
+    _id: String!
+    name: String!
     colorCode: String
     parentId: String
     relatedIds: [String]
     isGroup: Boolean

     createdAt: Date
   }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

backend/core-api/src/modules/tags/db/models/Tags.ts (3)

88-95: Return the removed tag (interface says ITagDocument); confirm orphaning semantics and relatedIds cleanup

Return the deleted document, not DeleteResult. Also, per PR summary you intend to orphan children by unsetting parentId—confirm removeRelatedTagIds removes this tag and its descendants from all ancestors to avoid stale relatedIds.

-      return models.Tags.deleteOne({ _id });
+      await models.Tags.deleteOne({ _id });
+      return tag;

Also applies to: 97-100

---

24-31: Harden parent validation: self-parent, not-found, and cycle prevention

Guard against setting parentId to self, distinguish “not found” vs “not a group”, and prevent cycles (parent is a descendant).

-      if (parentId) {
-        const parentTag = await models.Tags.findOne({ _id: parentId });
-
-        if (!parentTag?.isGroup) {
-          throw new Error('Parent tag must be a group');
-        }
-      }
+      if (parentId) {
+        if (_id && parentId === _id) {
+          throw new Error('A tag cannot be its own parent');
+        }
+        const parentTag = await models.Tags.findOne({ _id: parentId });
+        if (!parentTag) {
+          throw new Error('Parent tag not found');
+        }
+        if (!parentTag.isGroup) {
+          throw new Error('Parent tag must be a group');
+        }
+        if (_id && parentTag.relatedIds?.includes(_id)) {
+          throw new Error('Cannot set a tag’s parent to its descendant');
+        }
+      }

---

69-83: Handle parent moves: remove from old ancestry before adding to new

When parentId changes, you need to remove from the old parent chain, then add to the new one. Also, passing the updated doc to setRelatedTagIds is correct.

-      const updated = await models.Tags.findOneAndUpdate({ _id }, doc, {
-        new: true,
-      });
-
-      if (updated) {
-        await setRelatedTagIds(models, updated);
-      }
+      const updated = await models.Tags.findOneAndUpdate({ _id }, doc, { new: true });
+      if (updated) {
+        const parentChanged =
+          Object.prototype.hasOwnProperty.call(doc, 'parentId') &&
+          String(tag.parentId || '') !== String(updated.parentId || '');
+        if (parentChanged) {
+          // Remove from old ancestry chain
+          await removeRelatedTagIds(models, tag);
+        }
+        // Propagate to new ancestry chain (no-op if no parent)
+        await setRelatedTagIds(models, updated);
+      }

backend/core-api/src/commands/migrateContentType.ts (1)

33-37: Tags excluded from this migration — verify handoff and short-circuit when empty

Confirm that all tag-related migrations are fully handled by migrateTags.ts and that no consumers still expect tags to be processed here. Optionally, exit early if COLLECTIONS is empty to avoid a no-op run after connecting.

backend/core-api/src/modules/tags/db/models/Tags.ts (1)

60-66: Consider wrapping create + ancestry updates in a transaction

If consistency across multiple docs matters, run create + setRelatedTagIds in a MongoDB transaction to avoid partial ancestry updates on failures.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

backend/core-api/src/modules/tags/graphql/queries.ts (3)

9-9: Use correct FilterQuery generic (document type, not params)

Type the Mongo filter against the document shape.

-import { FilterQuery } from 'mongoose';
+import { FilterQuery } from 'mongoose';
+import { ITagDocument } from 'erxes-api-shared/core-types';
@@
-  const filter: FilterQuery<ITagFilterQueryParams> = { ...commonQuerySelector };
+  const filter: FilterQuery<ITagDocument> = { ...commonQuerySelector };

---

31-33: Allow explicit isGroup=false filtering

Guard for explicit boolean; current code skips false.

-  if (isGroup) {
-    filter.isGroup = isGroup;
-  }
+  if (typeof isGroup === 'boolean') {
+    filter.isGroup = isGroup;
+  }

---

23-25: Sanitize user input in regex to prevent ReDoS

Escape and bound searchValue; prefer $regex object.

-  if (searchValue) {
-    filter.name = new RegExp(`.*${searchValue}.*`, 'i');
-  }
+  if (searchValue) {
+    const ESCAPE_RE = /[.*+?^${}()|[\]\\]/g;
+    const safe = String(searchValue).slice(0, 100).replace(ESCAPE_RE, '\\$&');
+    filter.name = { $regex: safe, $options: 'i' };
+  }

backend/core-api/src/modules/tags/db/definitions/tags.ts (1)

8-8: Don’t enforce global unique on name; scope uniqueness

A field-level unique: true makes names globally unique across tenants/parents. Scope to (name, parentId, type) or include tenant key if multi-tenant.

-      name: { type: String, label: 'Name', unique: true },
+      name: { type: String, label: 'Name' },

Add a compound unique index (case-insensitive). If a standalone unique exists in prod, we must drop it in a migration before creating this one—I can draft it.

- tagSchema.index({ _id: 1, name: 1, parentId: 1, type: 1 });
+ // Query and uniqueness indexes
+ // Uniqueness within scope (add organizationId/workspaceId if multi-tenant)
+ tagSchema.index({ name: 1, parentId: 1, type: 1 }, { unique: true, collation: { locale: 'en', strength: 2 } });

backend/core-api/src/modules/tags/db/models/Tags.ts (2)

84-105: Handle parent moves correctly: remove stale ancestry only when parentId changes

Current code removes relatedIds based on presence of children, not on parent change, which leaves stale ancestry when moving a leaf and does extra work when editing unrelated fields.

Apply:

   public static async updateTag(_id: string, doc: ITag) {
     await this.validate(_id, doc);
 
-    const tag = await models.Tags.getTag(_id);
-
-    const childTags = await models.Tags.find({ parentId: tag._id });
-
-    if (childTags.length) {
-      await removeRelatedTagIds(models, tag);
-    }
+    const prev = await models.Tags.getTag(_id);
+    const parentChanged =
+      Object.prototype.hasOwnProperty.call(doc, 'parentId') &&
+      String(prev.parentId || '') !== String(doc.parentId || '');
+    if (parentChanged) {
+      await removeRelatedTagIds(models, prev);
+    }
 
-    const updated = await models.Tags.findOneAndUpdate(
-      { _id: tag._id },
+    const updated = await models.Tags.findOneAndUpdate(
+      { _id },
       doc,
       {
         new: true,
       },
     );
 
     if (updated) {
       await setRelatedTagIds(models, updated);
     }
 
     return updated;
   }

---

113-121: Child tags are orphaned on delete — confirm intentional and document behavior

Unsetting parentId on children leaves them top-level. If intentional (per PR), ensure callers and UI handle orphaned child tags; otherwise consider cascade delete or reassignment strategy.

Would you like a follow-up to add an optional cascading delete flag or a “reassign to parent’s parent” strategy?

backend/core-api/src/modules/tags/graphql/schemas.ts (1)

40-41: Fix tagsQueryCount signature: remove “type”, add isGroup (and optional parentId/instanceId)

Schema still exposes the removed argument.

-  tagsQueryCount(type: String, searchValue: String): Int
+  tagsQueryCount(searchValue: String, isGroup: Boolean, parentId: String, instanceId: String): Int

If you prefer strict parity, accept ${queryParams}-like args for count.

backend/core-api/src/modules/tags/graphql/queries.ts (1)

7-8: Avoid shadowing ids; dedupe collected IDs

The inner ids shadows the param and can confuse maintainers; also dedupe to prevent inflated $in arrays.

-  const { searchValue, parentId, ids, excludeIds, isGroup, type } = params;
+  const { searchValue, parentId, ids, excludeIds, isGroup, type } = params;
@@
-    let ids = [parentId, ...parentTag];
+    let collectedIds = [parentId, ...parentTag];
@@
-        ids = [...ids, ...childTag];
+        collectedIds = [...collectedIds, ...childTag];
@@
-    await getChildTags(parentTag);
+    await getChildTags(parentTag);
+    collectedIds = Array.from(new Set(collectedIds));
@@
-    filter._id = { $in: ids };
+    filter._id = { $in: collectedIds };

Also applies to: 38-54

backend/core-api/src/modules/tags/graphql/schemas.ts (2)

51-55: Optional: return a typed payload for tagsTag instead of JSON

Typed payload improves client ergonomics and schema discoverability.

 export const mutations = `
-  tagsAdd(name: String!, ${mutationParams}): Tag
-  tagsEdit(_id: String!, name: String, ${mutationParams}): Tag
-  tagsTag(targetIds: [String!]!, tagIds: [String!]!): JSON
+  tagsAdd(name: String!, ${mutationParams}): Tag
+  tagsEdit(_id: String!, name: String, ${mutationParams}): Tag
+  tagsTag(targetIds: [String!]!, tagIds: [String!]!): TagTagResult
   tagsRemove(_id: String!): JSON
 `;
+
+export const types = `
+  type TagTagResult {
+    modifiedCount: Int!
+    skippedCount: Int!
+  }
+  ${types}
+`;

Ensure resolver returns this shape.

---

3-21: Minor: consider non-nullables where guaranteed (future cleanup)

e.g., make _id and name non-nullable if guaranteed by DB. Low priority; align after migrations stabilize.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.